Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correction of BUG in subroutine parser and Support for multiple terminals #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marcsosduma
Copy link
Contributor

Support for multiple terminals – allows the Linux system to use not only XTerm but also gnome-terminal, konsole (KDE), and xfce4-terminal.

Bug in parser – in some cases, the snippet "// fix new codegen - new" was causing the parser to change the line number of the corresponding C code line after the "/* Program exit */" tag.

Copy link
Contributor

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have an adjusted setting to let the user choose?

src/mi2.ts Outdated
Comment on lines 1198 to 1223
if(this.isTerminalInstalled("xterm")){
this.createXtermTerminal(sleepVal, target);
}else if(this.isTerminalInstalled("gnome-terminal")){
this.createGNOMETerminal(sleepVal, target);
}else if(this.isTerminalInstalled("konsole")){
this.createKDETerminal(sleepVal, target);
}else if(this.isTerminalInstalled("xfce4-terminal")){
this.createXFCETerminal(sleepVal, target);
}
Copy link
Contributor

@GitMensch GitMensch Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be a configurable gdbtty to enable choosing between the terminals, otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Simon! Something like this? "gdbtty": "xfce4"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Simon! When you have a moment, take a look and see if it's okay.

Support for multiple terminals
Copy link
Contributor

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! I personally would prefer two commits - one for the parser fix and one for the new feature, but that's not my repo...

I do think that the parser fix should include a change in the testsuite.

@@ -67,7 +68,7 @@ export class SourceMap {
private version: string;
private lineBefore: string = "";
private performLine: number = -1; // 002 - stepOver in routines with "perform"
private isVersion2_2_or_3_1_1: boolean = false;
private isVersion2_2_or_3_1_1: boolean = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A trailing space slipped in here - otherwise it the changes LGTM.

@marcsosduma
Copy link
Contributor Author

Hello Simon! The tests pass, but I found the issue while debugging these files 'sample.cbl', 'subsample.cbl', and 'subsubsample.cbl'. When the system switches from one file to another, for example, from 'sample' to 'subsample', the debugger goes to the last line of code. With this correction, it seems that the problem is resolved.

@GitMensch
Copy link
Contributor

GitMensch commented Jan 16, 2024

I hop that @OlegKunitsyn may merge this soon.

In any case and "mostly unrelated" (at least unrelated to this repo, other than your changes being likely similar):
@marcsosduma Can I ask you to provide the terminal settings (gdb console, internal console, several external consoles) as a PR to https://github.com/WebFreak001/code-debug ? This is a quite important feature that I'm missing badly there.

@marcsosduma
Copy link
Contributor Author

Hello Simon!
Is this what you need?

launch.json:
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"name": "COBOL debugger",
"type": "gdb",
"request": "launch",
"cobcargs": [
"-free",
"-x"
],
"coverage": false,
"gdbtty": true,
"verbose": true,
"group": ["subsample.cbl","subsubsample.cbl"]
},
{
"name": "COBOL debugger attach local",
"type": "gdb",
"request": "attach",
"cobcargs": [
"-free",
"-x"
],
"pid": "${input:pid}"
},
{
"name": "COBOL debugger attach remote",
"type": "gdb",
"request": "attach",
"cobcargs": [
"-free",
"-x"
],
"remoteDebugger": "${input:remoteDebugger}"
}
]
}

image

@GitMensch
Copy link
Contributor

The configuration is fine - I've asked you to contribute similar code you've added to this extension (all gdbtty related ones) to another extension (that is for general GDB debugging).

@marcsosduma
Copy link
Contributor Author

Good morning, Simon! I can definitely try. It will be a pleasure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants